feat(fleet): co-own.sh removes exclusive ownership of a scope, provably - #47
Merged
jordonpeterson merged 3 commits intoAug 22, 2026
Merged
Conversation
Written first, per CONTRIBUTING: seven scenarios covering the exclusive-line removal, the amend-when-fallback-lacks-owner inversion, the nothing-behind refusal, and the untouched-repo guarantees (unchanged/skipped/dirty). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zaT8RgVZCfSEJshc2FpXS
A fleet-wide line like '.github/workflows @org/platform' cannot be un-exclusived by one policy file: the co-owners differ per repo. Deleting the line is right only when the broader rule already lists the owner; when it does not, deletion revokes instead of shares, so the script inverts and adds the broader team(s) to the line. Repos with nothing behind the line are refused untouched (exit 2) — someone must decide who co-owns. Every edit is kept only if verify --scope proves no out-of-scope path changed owners and the end state is owner-plus-at-least-one-other on every in-scope path. CI shellchecks the script alongside the existing ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zaT8RgVZCfSEJshc2FpXS
There was a problem hiding this comment.
🟡 Changes recommended
The new e2e TestMain leaks its temp dir due to os.Exit bypassing defers, and the script’s --scope handling can silently no-op on invalid/non-literal patterns instead of failing as a broken invocation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a fleet helper that updates per-repo CODEOWNERS so a given scope is never exclusively owned by a single team, while proving the edit is contained to the intended scope before reporting success.
Changes:
- Add
tools/fleet/co-own.shto co-own a scope by either removing an exclusive line or amending it usingcodeowners-tool, with rollback-on-failure behavior. - Add end-to-end Go tests that execute the script against real git repos and the real built
codeowners-toolbinary. - Document usage and add the script to CI ShellCheck coverage.
File summaries
| File | Description |
|---|---|
| tools/fleet/README.md | Documents intent, exit codes/output, and a sample fleet loop usage. |
| tools/fleet/coown_test.go | Adds e2e tests that build the real binary and run the script against temporary repos. |
| tools/fleet/co-own.sh | Implements the per-repo co-owning/verification/rollback logic and JSON status emission. |
| .github/workflows/ci.yml | Includes the new script in ShellCheck CI. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+28
to
+40
| func TestMain(m *testing.M) { | ||
| dir, err := os.MkdirTemp("", "coown-tool") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| defer os.RemoveAll(dir) | ||
| toolPath = filepath.Join(dir, "codeowners-tool") | ||
| build := exec.Command("go", "build", "-o", toolPath, "../../cmd/codeowners-tool") | ||
| if out, err := build.CombinedOutput(); err != nil { | ||
| panic("build codeowners-tool: " + err.Error() + "\n" + string(out)) | ||
| } | ||
| os.Exit(m.Run()) | ||
| } |
Comment on lines
+44
to
+46
| # Scope is compared against snapshot paths with slashes normalized away. | ||
| S="${SCOPE#/}"; S="${S%/}" | ||
| CFILE="" |
git 2.55 (on the CI runners) detaches maintenance after ordinary commands, so fixture commits leave a background process whose .git bookkeeping writes land at an arbitrary later moment. checkDirSnapshot hashed .git too, so the before/after comparison in the 'writes nothing' tests failed on whichever test the race hit — R-35b/R-35d one run, the R-33 skip and dry-run tests the next — while the tool wrote nothing. The invariant those tests state is about the working tree the tool can write to; .git internals were never its contract, so the snapshot now skips them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zaT8RgVZCfSEJshc2FpXS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure this prevents
A fleet rollout added
.github/workflows @org/platformto a hundred repos, and the intent has changed: platform should still own that scope, but together with whoever owns the surrounding tree — never alone. No single policy file can express that end state, because the co-owners differ per repo. Doing it withsedalone silently produces the two bad outcomes: deleting the line where the broader rule doesn't list the owner revokes their ownership instead of sharing it, and deleting it where nothing sits behind it leaves the scope unowned.What this adds
tools/fleet/co-own.sh— a per-repo script in the FLEET.md mold (cloning/PRs stay withgh), which:SCOPE OWNERline when the broader rule behind it already lists the owner (the clean answer);add_owner(scope, [...])list op (R-33b: one line change);verify --scopeproves no out-of-scope path changed owners and every in-scope path ends owned by the owner plus at least one other team;sync's 0/2/3 exit contract so the FLEET.md loop patterns apply unchanged.Repos that are already co-owned, have nothing in scope, or where the owner doesn't own the scope at all are left byte-identical (exit 0), so a second wave opens no empty PRs.
Tests
Written first, per CONTRIBUTING, and confirmed failing for the right reason before the script existed.
tools/fleet/coown_test.goruns the script against the real binary and real git repositories: line removal with out-of-scope ownership pinned, the amend inversion with the broader rule proven untouched, the nothing-behind refusal with HEAD/branch/tree restoration asserted, and the unchanged/skipped/dirty guarantees.gendocsonly sweepsinternal/*, sodocs/BEHAVIOR.mdis unaffected — verified withmake docs.go test -race ./...— all green (including the newtools/fleetpackage)make vet,make docs— clean, no driftshellcheck— clean; the script is added to CI's shellcheck list🤖 Generated with Claude Code
https://claude.ai/code/session_019zaT8RgVZCfSEJshc2FpXS
Generated by Claude Code